-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SVS: remove thumbnails and improve label/macro detection #4144
SVS: remove thumbnails and improve label/macro detection #4144
Conversation
…/macro images This is particularly necessary for certain types of anonymized/de-identified data, which may have a minimal comment that does not include a "label" or "macro" identifier.
In particular, this makes it much easier to remove a single resolution from a CoreMetadataList.
This controls whether the thumbnail image is included in the pyramid. The default is "true", which removes the thumbnail image entirely.
This should cover the cases where: - no comment is present at all - a comment is present an correctly identifies the label/macro - a comment is present but does not identify the label/macro - a label or macro is identified and has a larger IFD index than the removed thumbnail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested against the representative SVS sample files in the OME QA repository. For each sample file, the output of showinf -noflat -omexml
was compared between:
- Bio-Formats 7.1.0
- a build of the Bio-Formats command-line tools with this PR included
- a build of the Bio-Formats command-line tools with this PR included and
-option svs.remove_thumbnail
set
In addition, for each SVS sample including at least a label and/or a macro file, the ancillary series were inspected visually by passing -series {1,2}
With regards to macro/label images, everything was found to be unchanged compared to Bio-Formats 7.1.0. This matches the fact that the nightly CI builds are passing and the associated configuration changes did not modify the label/macro images. All macro/label images inspected manually were correct validating further the fact that the SVS macro/label assignment has been resolved in #4081 to the best of our knowledge
The additional logic introduced to handle macro/label images with removed ImageDescription was found to have no negative impact on our SVS samples, matches the expectations of the official Aperio SVS specification and gives extra robustness for SVS files that might have been modified by a third-party de-identification script. It is worth noting that in this scenario, the reader no longer has an unambiguous way to detect extra series as label or macro images without inspecting the binary data.
With regards to the SVS thumbnail, the logic removing the thumbnail from the resolutions by default worked as expected for all samples and the new reader option allows to restore the current behavior where the thumbnail is the lowest part of the resolution.
The Bio-Formats API imposes limited constraints on the pyramidal levels. From a strict API perspective, the thumbnail of SVS files could be treated as yet another resolution level. Instead there are a few functional limitations that motivated the removal of the thumbnail from the resolution list:
- the downsampling factor is generally consistent between consecutive levels of the pyramid but the factor between the smallest non-thumbnail resolution and the thumbnail is usually different,
- in the most extreme case, a SVS file could include only a single large-resolution level and a thumbnail and would be currently detected as multi-resolution. As discussed in the accompanying issue and forum threads, visualizing such data e.g. in OMERO or QuPath would be lead to a poor experience. With this change, such a SVS file would instead be reported as a single-resolution image and suggest more strongly that a conversion to an appropriate multi-resolution format
- another extreme use case found across our samples was a SVS file with a thumbnail dimensions is 1x1. Similar to the above, it makes complete sense to remove this from the whole slide image resolution list
- finally, removing the thumbnail from the resolution lists is also consistent with the OpenSlide behavior which treats the thumbnail as an associated image - https://github.com/openslide/openslide/blob/b2f942269ba6ccdda6bd860aae4f4260ac02ef70/src/openslide-vendor-aperio.c#L502-L505
The addition of the reader option also adds some form of backwards-compatibility for consumer code which would have been explicitly relying on the last resolution of the SVS whole slide image being the thumbnail.
Overall, I am happy for these changes to be included in the upcoming 7.2.0 minor release. The rationale and the implications of the thumbnail removal should likely be assessed independently by the @ome/formats team and we can discuss further at the weekly Monday meeting if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall Im happy with the approach used here of removing the thumbnails. The rationale for it outlined in the original Issue and in Sebs review make sense to me. Additionally having the option to revert to the old behaviour also allows it to remain backwards compatible.
Manually testing some of the impacted sample files showed the thumbnail being correctly removed. Otherwise the images and metadata all looked to be correct and unaffected. Testing the new option also showed it correctly reverting to include the thumbnails again.
We will need a follow up docs PR to document the new reader option but otherwise this PR looks good from my side.
Fixes #3757.
As discussed at the end of #3757, thumbnail images are now removed by default. The
svs.remove_thumbnail
option controls this behavior; it isfalse
by default but setting totrue
should cause thumbnails to be included (matching behavior without this PR).816a588 and 2fd4489 have related updates to label/macro detection, so that the corresponding image names are set correctly for either value of
svs.remove_thumbnail
. Those commits also add support for the case where a label or macro image has a non-null comment that does not includelabel
ormacro
; this is typically the result of anonymization/de-identification.Configuration changes are needed to remove the thumbnails; corresponding PRs are forthcoming.